Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issue 825 #826

Merged
merged 7 commits into from
May 6, 2024
Merged

Fixing issue 825 #826

merged 7 commits into from
May 6, 2024

Conversation

AbdullahFaqeir
Copy link
Contributor

Fixing an issue caused import command or any other process that deals with the collection to recreate the collection as the exists flags wasn't updated properly.

Related Issue
Other Related Issue
Previous PR

@driesvints
Copy link
Member

I'd give much more detailed info here for Taylor to review. Otherwise it won't be clear why this is needed.

@taylorotwell
Copy link
Member

But now the method says it returns a Collection in the docblock but actually returns an index?

@taylorotwell taylorotwell marked this pull request as draft April 30, 2024 12:44
@AbdullahFaqeir
Copy link
Contributor Author

But now the method says it returns a Collection in the docblock but actually returns an index?

I apologise as this is a mess-naming for the variable, will make sure to fix this.

@stammbach
Copy link
Contributor

Hey there, I would really appreciate this getting in asap. The latest version makes Scout + Typesense unusable with "large" datasets over 500 items. First time trying Scout & this makes onboarding really hard.

The previous version or applying this fix works as expected.

@driesvints
Copy link
Member

@stammbach feel free to send in another PR that we can merge in faster.

@stammbach
Copy link
Contributor

@driesvints I've included the fix in one of my PRs becuase I touched that line anyway. Not sure if this makes anything faster but would really appreciate you checking it out.

Fix variable name.
@AbdullahFaqeir
Copy link
Contributor Author

@taylorotwell I apologise for taking too long to fix this, I've committed a fix to the variable name.

@driesvints
Copy link
Member

@AbdullahFaqeir Taylor doesn't sees draft PR's. Please mark this as ready if you need another review.

@AbdullahFaqeir AbdullahFaqeir marked this pull request as ready for review May 3, 2024 21:35
@taylorotwell taylorotwell merged commit 8146bf4 into laravel:10.x May 6, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants